Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Control round and decimal places in Gauge Visualization when using aggregate functions like average #91293

Merged
merged 20 commits into from
Mar 1, 2021
Merged

Control round and decimal places in Gauge Visualization when using aggregate functions like average #91293

merged 20 commits into from
Mar 1, 2021

Conversation

VladLasitsa
Copy link
Contributor

@VladLasitsa VladLasitsa commented Feb 12, 2021 •
edited
Loading

Closes: #89404

Summary

Previously we don't have possibility to provide format pattern when use percentage mode in some visualizations. We can change pattern only globally using advanced settings. It is not user-friendly. Because of this I added a new input field which allow to set pattern for visualization in percentage mode. If you don't set some pattern will be use default pattern. Now we have three type of visualizations which use percentage mode. I updated all of them:

  • vis_type_metric
  • vislib heatmap
  • vislib gauge

Below you can see how it works with my changes:

Elastic (1)

Release notes

Adds possibility to provide format pattern in percentage mode in metric, heatmap and gauge visualizations.

Checklist

Delete any items that are not applicable to this PR.

  • Documentation was added for features that require explanation or tutorials
  • Unit or functional tests were updated or added to match the most common scenarios
  • Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
  • This renders correctly on smaller devices using a responsive layout.

@VladLasitsa VladLasitsa changed the title Add field for providing format in percentage mode Control round and decimal places in Gauge Visualization when using aggregate functions like average Feb 12, 2021
@VladLasitsa
Copy link
Contributor Author

@elasticmachine merge upstream

@VladLasitsa VladLasitsa self-assigned this Feb 19, 2021
@VladLasitsa VladLasitsa added release_note:feature Makes this part of the condensed release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 v7.12.0 labels Feb 19, 2021
@VladLasitsa VladLasitsa marked this pull request as ready for review February 19, 2021 14:01
@VladLasitsa VladLasitsa requested a review from a team February 19, 2021 14:01
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@VladLasitsa
Copy link
Contributor Author

@elasticmachine merge upstream

@alexwizp alexwizp added v7.13.0 and removed v7.12.0 labels Feb 23, 2021
stratoula
stratoula reviewed Feb 23, 2021
View reviewed changes
src/plugins/vis_default_editor/public/components/options/percentage_mode.tsx Outdated Show resolved Hide resolved
src/plugins/vis_type_vislib/public/to_ast.ts
@@ -78,7 +78,10 @@ export const toExpressionAst = async <TVisParams extends VisParams>(
}
}
if (visConfig?.gauge?.percentageMode === true) {
yDimension.format = { id: 'percent' };
yDimension.format = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question because it is not clear to me, don't we need to also do it for heatmap?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't use it for heatmap. Only in gauge.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the percentage mode also exists for heatmaps too 🤔

@stratoula
Copy link
Contributor

And something more, I found it to be a great enhancement but I think it is not something that I would label as a feature cc @alexwizp

@VladLasitsa
Copy link
Contributor Author

@elasticmachine merge upstream

@stratoula stratoula added release_note:enhancement and removed release_note:feature Makes this part of the condensed release notes labels Feb 25, 2021
@VladLasitsa
Copy link
Contributor Author

@elasticmachine merge upstream

@VladLasitsa
Copy link
Contributor Author

@elasticmachine merge upstream

stratoula
stratoula approved these changes Mar 1, 2021
View reviewed changes
Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I didn't test it again but it seems to work fine.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
visDefaultEditor 126 127 +1
visTypeVislib 262 263 +1
total +2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
visTypeVislib 623.1KB 624.6KB +1.6KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
visDefaultEditor 34.0KB 36.1KB +2.1KB
visTypeMetric 22.8KB 22.8KB +41.0B
visTypeVislib 36.0KB 36.2KB +203.0B
total +2.3KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @VladLasitsa

@stratoula
Copy link
Contributor

@elasticmachine run elasticsearch-ci/docs

@VladLasitsa VladLasitsa merged commit ccf1fcc into elastic:master Mar 1, 2021
VladLasitsa added a commit to VladLasitsa/kibana that referenced this pull request Mar 1, 2021
@VladLasitsa @kibanamachine
Control round and decimal places in Gauge Visualization when using ag…
f3964da 
…gregate functions like average (elastic#91293)

* Add field for providing format in percentage mode

* Fix CI

* Add tests

* Fix ci

* Fix some remarks

* Fix ci

* Fix some remarks

* Fix ci

* Fix width

* Fix some remarks

* Fix text

* Fix i18n

* Remove unneeded import

* Remove unneeded code

Co-authored-by: Kibana Machine <[email protected]>
@VladLasitsa VladLasitsa mentioned this pull request Mar 1, 2021
Merged
VladLasitsa added a commit that referenced this pull request Mar 1, 2021
@VladLasitsa @kibanamachine
Control round and decimal places in Gauge Visualization when using ag…
0772e20 
…gregate functions like average (#91293) (#93014)

* Add field for providing format in percentage mode

* Fix CI

* Add tests

* Fix ci

* Fix some remarks

* Fix ci

* Fix some remarks

* Fix ci

* Fix width

* Fix some remarks

* Fix text

* Fix i18n

* Remove unneeded import

* Remove unneeded code

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
jloleysens added a commit that referenced this pull request Mar 3, 2021
@jloleysens
Merge branch 'ilm/rollup-v2-action' of github.com:elastic/kibana into…
cae0462 
… ilm/rollup-v2-action

* 'ilm/rollup-v2-action' of github.com:elastic/kibana:
  [Security Solution][Case][Bug] Only add rule object for alert comments (#92977)
  [Security Solution][Case] Show the current connector name in case view (#93018)
  [Security Solution] Remove unused mock data (#92357)
  Adds mapping to the signals for the indicator rules that were missing (#92928)
  skip flaky suite (#85208)
  Cleanup spaces plugin (#91976)
  Control round and decimal places in Gauge Visualization when using aggregate functions like average (#91293)
  Added alerting ui mock for jest test (#92604)
  Remove "beta" label from URL Drilldown as it is now GA (#92859)
@stratoula stratoula mentioned this pull request Mar 11, 2021
Closed
@timroes timroes added the Feature:Gauge Vis Gauge and goal visualization label Apr 26, 2021
@andyharless
Copy link

Are there plans to implement this feature for gauges that display raw aggregations rather than percentages?

@stratoula
Copy link
Contributor

This is already possible in Lens gauges, we don't plan to add more capabilities on the aggbased charts.

image

@andyharless
Copy link

It would be nice to be able to control decimal places for arc-style gauges, but I guess that's not happening any time soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gchaps gchaps gchaps left review comments

@sulemanof sulemanof sulemanof approved these changes

@alexwizp alexwizp alexwizp approved these changes

@stratoula stratoula stratoula approved these changes

@nickofthyme nickofthyme Awaiting requested review from nickofthyme

Assignees

@VladLasitsa VladLasitsa

Labels
Feature:Gauge Vis Gauge and goal visualization release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.13.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Control round and decimal places in Gauge Visualization when using aggregate functions like average
9 participants
@VladLasitsa @elasticmachine @stratoula @kibanamachine @andyharless @alexwizp @sulemanof @gchaps @timroes